-
Notifications
You must be signed in to change notification settings - Fork 11.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[9.x] Implement Symfony Mailer #38481
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A lot of duplicates with $this->email->method
.
Maybe it's better to create a separate method with passing cc
, bcc
, etc?
dbbaaeb
to
b3ed86f
Compare
@michael-rubel it's fine as is I think. |
b5895d8
to
4bc725b
Compare
I don't think we can remove this: The Postmark X-PM-Message-Stream header won't be set anymore (see [8.x] Make it possible to set Postmark Message Stream ID #35755) If the Symfony version of the Postmark driver doesn't support it we should probably just write our own Postmark transport for Symfony Mailer. |
You probably would only need to change the There you would need to add the key If you were using the Postmark SMTP Transport it would still be possible to use the header like before. (https://postmarkapp.com/developer/user-guide/send-email-with-smtp in the Section Connection-Details) Edit: Maybe doing a PR to Symfony would be an idea. |
@driesvints Regarding the Postmark X-PM-Message-Stream header. The current Swift Mailer Implementation of the Postmark Transport also works with the API. There it also just attaches the Header to the request and it seems to be working fine right now. Edit: I also can't find any documentation regarding this. This could change anytime if Postmark would want to. |
420e7d3
to
40ba715
Compare
@Jubeki thanks for all your investigation. I did some of my own and it turns out that Message Streams are a documented feature after all. I've sent a PR to Symfony to provide support for it and all the details are in the PR: symfony/symfony#42941 |
@driesvints yeah it is documented for sure. What I meant was that the current implementation (Laravel 8.x) uses the X-PM-Message-Stream header together with the Postmark API. I only found documentation for the Header in the SMTP Section of Postmark and not in the API Section. So I was just wondering how the current Implementation is even working as intended. |
@Jubeki I guess Postmark both accepts a |
Also maybe another concern which should be addressed: What if somebody sets the X-PM-Message-Stream header on a email instance. Which value will be choosen? With the new implementation in 9.x there is both the header and the json entry present. Which has a higher priority? |
33eecce
to
0d8e72e
Compare
Co-authored-by: Julius Kiekbusch <jubeki99@gmail.com>
* Create SentMessage wrapper for Symfony's SentMessage * Wrap Symfony SentMessage * Update Docblocks to Illuminate\Mail\SentMessage * Fix sendMailable * Update SentMessage.php Co-authored-by: Dries Vints <dries@vints.io>
@taylorotwell @Jubeki has added the |
@Jubeki thanks for all your help with this! ❤️ |
I also have to say thanks to you @taylorotwell and @driesvints. You are doing really great work with Laravel. Learning by doing is the best way to learn for me. Even more so if you don't want to work on your bachelor thesis or learn for your exams. 😂 |
Thanks a lot for all your help @Jubeki! |
@driesvints hey, i think there could be issue with this. Mail::to(['me@gmail.com'])->send(new test()); and i get this error: I also tried from symfony docs: Mail::to(new Address('me@gmail.com'))->send(new test()); But then i get |
@dyanakiev I just tried your example on a fresh Laravel 9 App and it seems to be working fine. Can you please provide a repository? If you have GitHub CLI:
If you don't have GitHub CLI:
Then add your changes and commit them. You can also take a look at: https://github.com/Jubeki/bug-symfony-mailer
Then take a look in |
Method Illuminate\Mail\Mailer::getSwiftMailer does not exist. i have error please give me solution |
This PR replaces SwiftMailer with Symfony Mailer. Special thanks to @Jubeki for his help with this PR.
See https://symfony.com/blog/the-end-of-swiftmailer
Todo
symfony/amazon-mailer
symfony/mailgun-mailer
symfony/postmark-mailer
Breaking changes
send
,html
,text
&plain
don't return the number of recipients anymore but an instance ofIlluminate\Mail\SentMessage
which contains theSymfony\Component\Mailer\SentMessage
instanceSwift
methods were renamed toSymfony
Illuminate\Mail\Message
class now contains an instance ofSymfony\Component\Mime\Email
instead ofSwift_Message
so all forwarding calls in userland will need to be updatedauth_mode
in the mail config was removed (see [Mailer] Remove the auth mode DSN option and support in the eSMTP transport symfony/symfony#33237)smtp
key. Instead you need to set the options directly in the config. For example, when disabling TLS peer verification (see [Mailer] add ability to disable the TLS peer verification via DSN symfony/symfony#35262)mime.idgenerator.idright
. This is no longer possible with Symfony Mailer. Instead, Symfony Mailer will automatically generate a Message ID based on the sender and add it to a message automatically (see [9.x] Implement Symfony Mailer #38481 (comment))Possible improvements
We could wrapSymfony\Component\Mailer\SentMessage
to add some nice API methods like retrieving recipients, etcMAILER_DSN
strings to set up transports. This will allow much more thorough configuration for transports and we could at the same time provide BC support for the old configuration schemes.Design Choices
Symfony Mailer vs Symfony Transport
Previously a
Swift_Mailer
instance was provided to the laravelMailer
instance. I didn't opt to instantiate a SymfonyMailer
instance because in some situations you'd need to interact with the transport (like the array and log transport) and there's no getter for the transport on the SymfonyMailer
instance. Since we don't use the Symfony logger or events functionality, I decided to pass theTransportInterface
instead. You can think of Laravel'sMailer
class to be a direct replacement for Symfony'sMailer
class.Docs
laravel/docs#7306